[#346] Add Baseline Documentation #347
Conversation
DVidal1205
added
Documentation
📝 WalkthroughWalkthroughAdds four new documentation guides (Getting Started, Architecture, API & Permissions, GitHub Etiquette); updates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/API-AND-PERMISSIONS.md`:
- Line 469: Update the phrasing that currently reads "Read our [Github
Etiquette](./GITHUB-ETIQUETTE.md) guide for how to contribute to the project" to
use the correct brand capitalization "GitHub" (i.e., "Read our [GitHub
Etiquette](./GITHUB-ETIQUETTE.md) guide...") so the link text and any
occurrences of "Github" in this sentence are replaced with "GitHub".
In `@docs/ARCHITECTURE.md`:
- Around line 282-285: Update the link text "Github Etiquette" to the correct
brand spelling "GitHub Etiquette" in the ARCHITECTURE.md list item that
references the ./GITHUB-ETIQUETTE.md page (the bullet starting with "Read our
[Github Etiquette](./GITHUB-ETIQUETTE.md) guide"), leaving the link target
unchanged.
In `@docs/GETTING-STARTED.md`:
- Around line 244-246: Update the link text in GETTING-STARTED.md from "Github
Etiquette" to the correct brand spelling "GitHub Etiquette" so the list item "-
Read our [Github Etiquette](./GITHUB-ETIQUETTE.md) guide..." becomes "- Read our
[GitHub Etiquette](./GITHUB-ETIQUETTE.md) guide..."; modify the display text
only (leave the file path GITHUB-ETIQUETTE.md unchanged).
In `@package.json`:
- Around line 14-17: The db:pull npm script currently includes the literal
placeholder "[--truncate]" which is passed into process.argv and prevents the
script's args.includes("--truncate") check in scripts/get_prod_db.ts from ever
matching; remove the bracketed placeholder from the "db:pull" script so it reads
simply "pnpm --filter=@forge/db with-env tsx scripts/get_prod_db.ts" and rely on
users to supply flags at invocation (e.g. pnpm db:pull --truncate), keeping the
scripts/get_prod_db.ts logic (args.includes("--truncate")) unchanged.
In `@packages/db/src/schemas/knight-hacks.ts`:
- Around line 80-83: The schema change adjusting profilePictureUrl length (field
profilePictureUrl in packages/db/src/schemas/knight-hacks.ts) must be applied
via a SQL migration instead of db:push: enable drizzle-kit generate, point its
outDir to a version-controlled migrations folder (e.g.,
packages/db/migrations/), run drizzle-kit generate after modifying schema (so it
emits a migration that alters profile_picture_url length and any related changes
involving shirtSize/shirtSizeEnum), commit the generated migration, and update
CI/deploy scripts to run drizzle-kit (or the DB migration runner) on startup
instead of using drift/db:push so migrations are applied in production and
reversible/auditable.
🧹 Nitpick comments (1)
docs/API-AND-PERMISSIONS.md (1)
421-428: Add a language identifier to the fenced block.Markdownlint flags this fence as missing a language; using
text(orbashif it’s a shell tree) improves readability and lint compliance.✅ Suggested fix
-``` +```text routers/ ├── members.ts # Member management ├── events.ts # Event management ├── roles.ts # Role/permission management ├── misc.ts # Form integrations and misc └── index.ts # Main router that combines all</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
DGoel1602
left a comment
DGoel1602
left a comment
There was a problem hiding this comment.
Not a real review just double checking some info
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/API-AND-PERMISSIONS.md`:
- Around line 421-428: Update the fenced code block that shows the "routers/"
tree so the opening fence is labeled with a language (e.g., change ``` to
```text); this addresses the MD040 markdownlint warning and improves rendering
for the block containing the lines starting with "routers/" and the entries
"members.ts", "events.ts", "roles.ts", "misc.ts", and "index.ts".
azizu06
left a comment
azizu06
left a comment
There was a problem hiding this comment.
As an onboarding new dev on the team this documentation really helped me to understand the structure of forge and the ideal workflows I should follow as I start contributing. In my review I left some small suggestions as well as clarifying questions.
| > IMPORTANT! | ||
|
|
||
| You must push the database schema to your local database before running the project. This is a common source of errors for new contributors. | ||
|
|
||
| ```bash | ||
| pnpm db:push |
There was a problem hiding this comment.
Are there any examples of what error messages may be displayed if I forget to do this step? Would help me be more aware of avoiding this error and catching it.
There was a problem hiding this comment.
biggest one is failed to get session. will add now, good idea :)
mchdich
left a comment
mchdich
left a comment
There was a problem hiding this comment.
Everything looks pretty straightforward. Will just have to get used to the codebase and workflow
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/GETTING-STARTED.md`:
- Line 90: Update the Markdown link that currently uses HTTP to use HTTPS:
replace the text "[local.drizzle.studio](http://local.drizzle.studio)" with
"[local.drizzle.studio](https://local.drizzle.studio)" so the line reading "This
opens a GUI at [local.drizzle.studio](http://local.drizzle.studio)." becomes
"This opens a GUI at [local.drizzle.studio](https://local.drizzle.studio)."
🧹 Nitpick comments (1)
docs/GETTING-STARTED.md (1)
115-115: Use obviously fake Discord IDs in the example.While Discord IDs are public, the example IDs could be mistaken for real production values. Using clearly fake IDs makes it obvious these are placeholders that users should replace.
📝 Clearer example with fake IDs
-pnpm db:bootstrap 1321955700540309645 238081392481665025 +pnpm db:bootstrap 123456789012345678 987654321098765432
| 2. Create a new application | ||
| 3. Get your Client ID and Client Secret | ||
| 4. Add these to your `.env` file | ||
|
|
There was a problem hiding this comment.
Is there a specific Redirect URI for OAuth2 in the Discord app? Or is it something like http://localhost:PORT/api/auth/callback/discord?
6 participants
Why
Our docs weren't the best on Notion. They were also gatekept, and closed source, making OSS devs on Forge frustrated.
What
Issue(s): #346
Adds the docs we definitely totally already had on Notion ( 💀 ) to
./docsfolder for public access! Details both Dev Team and non-team member contribution practices and common gotchas.Test Plan
Read the docs... I guess
Checklist
db:pushbefore mergingSummary by CodeRabbit